YARN-10949. Simplify AbstractCSQueue#updateMaxAppRelatedField and fin…#3500
YARN-10949. Simplify AbstractCSQueue#updateMaxAppRelatedField and fin…#3500shuzirra merged 7 commits intoapache:trunkfrom
Conversation
…d a more meaningful name for this method
|
💔 -1 overall
This message was automatically generated. |
...n/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/capacity/AbstractCSQueue.java
Outdated
Show resolved
Hide resolved
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
...rc/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/capacity/LeafQueue.java
Outdated
Show resolved
Hide resolved
tomicooler
left a comment
There was a problem hiding this comment.
Thanks for working on this. Non-binding +1.
| int maxGlobalApplications = conf.getGlobalMaximumApplicationsPerQueue(); | ||
| int maxSystemApplications = conf.getMaximumSystemApplications(); | ||
| int baseMaxApplications = maxGlobalApplications > 0 ? | ||
| maxGlobalApplications : maxSystemApplications; |
There was a problem hiding this comment.
I think, this should be more like a Math.min situation, if the maxGlobal is defined, it is a per queue limit, while max System is the total number of applications, so if System is set, it should be always a limiting factor. Also if System is too high, then maxGlobal should be the limiting factor, so it's more like
maxGlobalApplications > 0 ? Math.min(maxGlobalApplications, maxSystemApplications) : maxSystemApplications;
There was a problem hiding this comment.
I agree on this and this change still conforms to our existing tests.
There was a problem hiding this comment.
The problem with this approach is that system max app defaults to 10000. This means you basically can not set maxGlobalApp higher than this value, because it will be trimmed to 10000. We could pursue this path by making maxSystemApp defaults to -1 and check if it is defined, but I am not convinced it is worth the effort.
There was a problem hiding this comment.
According to the documentation:
Maximum number of applications in the system which can be concurrently active both running and pending. Limits on each queue are directly proportional to their queue capacities and user limits. This is a hard limit and any applications submitted when this limit is reached will be rejected. Default is 10000
This means to me, that maxSystemApp should be an absolute upper limit, if the user wishes to go above it, they should raise this limit. MaxGlobalApp should be always <= maxSystem app. But look at this the following way: if the user has 100 queues with maxGlobalApp 1000, and they saturate 10 of them with 10000 applications, then they won't be able to start any application in the 11th queue, because of the maxSystem is reached. So this limitation is already in place. As it should be.
See:
So capacity scheduler will enforce the max system application anyway, even if the leaf queue would allow it, so there is no point in allowing more application per leaf queue than the system wide max applications.
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
…d a more meaningful name for this method
Description of PR
How was this patch tested?
For code changes:
LICENSE,LICENSE-binary,NOTICE-binaryfiles?